Extract GCEPD pv tests #40885

copejon · 2017-02-02T20:19:07Z

What this PR does / why we need it:
This is strictly a refactor moving the GCEPD suite in Persistent Volumes E2E to it's own file. This will make future provider specific additions to pv testing much more organized and readable. It will also enable a smoother transition to providers moving out of tree by consolidating related tests.

NONE

k8s-ci-robot · 2017-02-02T20:19:08Z

Hi @copejon. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with @k8s-bot ok to test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

k8s-reviewable · 2017-02-02T20:19:13Z

This change is

copejon · 2017-02-02T20:19:18Z

cc @jeffvance

jeffvance · 2017-02-02T20:40:51Z

test/e2e/persistent_volumes.go

@@ -72,6 +71,19 @@ func completeMultiTest(f *framework.Framework, c clientset.Interface, ns string,
 	deletePVCandValidatePVGroup(c, ns, pvols, claims)
 }

+// Delete the nfs-server pod.
+func nfsServerPodCleanup(c clientset.Interface, config VolumeTestConfig) {


Is it better to move this to pvutil.go?

jeffvance · 2017-02-02T20:42:47Z

test/e2e/pvutil.go

@@ -62,19 +62,6 @@ type persistentVolumeConfig struct {
 	namePrefix string
 }

-// Delete the nfs-server pod. Only done once per KubeDescription().
-func nfsServerPodCleanup(c clientset.Interface, config VolumeTestConfig) {


why move this func? Seems that any other provider that wants to run nfs to test their storage will want this func,

As it's written now, NFS has been an abstract way to provide a hostPath volume. Provider specific tests (e.g. GCE) define PVs that use the vendor's PersistentVolumeSource (like GCEPersistentDiskVolumeSource). So I was thinking that it's only relevant to our NFS & disruptive suite.

Today I see nfsServerPodCleanup called from these 2 files:
. test/e2e/persistent_volumes.go
. test/e2e/persistent_volumes-disruptive.go
and defined in pvutil.go.
So, since it is called by 2 pv related files, I still don't understand why it should not remain in pvutil.go...

Okay, given that, I think you're right. Will move back to pvutil

madhusudancs · 2017-02-02T21:01:17Z

I am not the right person to review this.

cc @kubernetes/sig-storage-pr-reviews

rootfs · 2017-02-02T21:32:38Z

test/e2e/persistent_volumes-gce.go

@@ -0,0 +1,138 @@
+/*
+Copyright 2015 The Kubernetes Authors.


rootfs · 2017-02-02T21:37:08Z

test/e2e/persistent_volumes-gce.go

+
+// Testing configurations of single a PV/PVC pair attached to a GCE PD
+var _ = framework.KubeDescribe("PersistentVolumes:GCEPD", func() {
+


jeffvance · 2017-02-02T23:57:20Z

It lgtm...

rootfs · 2017-02-03T13:42:48Z

test/e2e/persistent_volumes.go

@@ -84,9 +96,6 @@ var _ = framework.KubeDescribe("PersistentVolumes [Volume][Serial]", func() {
 		ns = f.Namespace.Name
 	})

-	///////////////////////////////////////////////////////////////////////


keep these lines

rootfs · 2017-02-03T13:43:29Z

test/e2e/pvutil.go

@@ -62,19 +62,6 @@ type persistentVolumeConfig struct {
 	namePrefix string
 }

-// Delete the nfs-server pod. Only done once per KubeDescription().


keep nfs functions

See below re: provider segregation.

As the primary persistent_volumes.go file will exclusively test against an NFS volume, it made more sense to keep NFS util code with those tests.

rootfs · 2017-02-03T13:45:47Z

@copejon can you share more thought why extracting gce from persistent volumes and the works going forward depending on this refactoring? As none of other e2e test files are specific to a single cloudprovider, they share core functions among supported cloud.

copejon · 2017-02-03T15:22:33Z

@rootfs This refactor is an attempt to preempt a situation where test suites for different providers are all clumped together in one file with the goal of improving structure and readability. We're beginning to see more tests coming in for PVs that are provider specific. GCE is already singled out; vSphere is now seeing targeted PV testing with #40756.

Secondly, having each provider test and accompanying utility code in their own file will help to smooth transition as vendors move out of tree.

rootfs · 2017-02-03T16:09:10Z

SGTM, please rebase

jeffvance · 2017-02-21T21:28:34Z

test/e2e/persistent_volumes.go

-	pv, pvc := createPVPVC(c, pvConfig, ns, isPrebound)
-	waitOnPVandPVC(c, ns, pv, pvc)
+// Delete the nfs-server pod.
+func nfsServerPodCleanup(c clientset.Interface, config VolumeTestConfig) {


I don't see any callers of this func, so it should be deleted unless I missed one...

jeffvance · 2017-02-21T21:33:50Z

/approve

copejon · 2017-02-23T18:22:56Z

@k8s-bot test this

mikedanese · 2017-02-23T20:07:01Z

/approve

copejon · 2017-02-27T21:12:48Z

@k8s-bot bazel test this

kerneltime · 2017-03-02T17:48:41Z

cc @divyenpatel

jeffvance · 2017-03-02T18:18:21Z

/lgtm

copejon · 2017-03-13T15:09:04Z

@k8s-bot cvm gce e2e test this

jeffvance · 2017-03-28T16:05:05Z

@k8s-bot verify test this

jeffvance · 2017-03-28T16:05:15Z

/lgtm

k8s-github-robot · 2017-03-28T16:07:35Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: copejon, jeffvance, mikedanese

Needs approval from an approver in each of these OWNERS Files:

~~test/OWNERS~~ [mikedanese]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

k8s-github-robot · 2017-03-28T17:34:08Z

Automatic merge from submit-queue

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Feb 2, 2017

copejon changed the title ~~Extracted GCEPD pv tests~~ Extract GCEPD pv tests Feb 2, 2017

copejon force-pushed the refactor-gce-pv-tests branch from 39e665c to 26c612a Compare February 2, 2017 20:22

k8s-github-robot assigned madhusudancs Feb 2, 2017

k8s-github-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. release-note-none Denotes a PR that doesn't merit a release note. labels Feb 2, 2017

copejon mentioned this pull request Feb 2, 2017

e2e tests for vSphere cloud provider #40756

Merged

jeffvance reviewed Feb 2, 2017

View reviewed changes

madhusudancs removed their assignment Feb 2, 2017

k8s-github-robot assigned spxtr Feb 2, 2017

rootfs reviewed Feb 2, 2017

View reviewed changes

test/e2e/persistent_volumes-gce.go Outdated

@@ -0,0 +1,138 @@

/*

Copyright 2015 The Kubernetes Authors.

Copy link

Contributor

rootfs Feb 2, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2017

rootfs reviewed Feb 2, 2017

View reviewed changes

copejon force-pushed the refactor-gce-pv-tests branch from 26c612a to 35687cd Compare February 2, 2017 21:51

spxtr assigned jeffvance and rootfs and unassigned spxtr Feb 2, 2017

rootfs reviewed Feb 3, 2017

View reviewed changes

copejon force-pushed the refactor-gce-pv-tests branch from 35687cd to 897afcc Compare February 3, 2017 20:02

k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 3, 2017

jeffvance reviewed Feb 21, 2017

View reviewed changes

copejon force-pushed the refactor-gce-pv-tests branch from 163e086 to 4a28a1e Compare February 22, 2017 17:41

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 23, 2017

copejon force-pushed the refactor-gce-pv-tests branch from 4a28a1e to 18cb2db Compare February 23, 2017 19:58

k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 23, 2017

k8s-github-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Feb 23, 2017

copejon force-pushed the refactor-gce-pv-tests branch from 18cb2db to f5a718c Compare February 27, 2017 16:10

k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 27, 2017

copejon force-pushed the refactor-gce-pv-tests branch from f5a718c to 8b71f46 Compare February 28, 2017 16:14

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 2, 2017

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 23, 2017

Extracted GCEPD pv tests

29d4c1c

copejon force-pushed the refactor-gce-pv-tests branch from 8b71f46 to 29d4c1c Compare March 28, 2017 14:54

k8s-github-robot removed lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Mar 28, 2017

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 28, 2017

k8s-github-robot merged commit b890d31 into kubernetes:master Mar 28, 2017

copejon deleted the refactor-gce-pv-tests branch April 12, 2017 20:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extract GCEPD pv tests #40885

Extract GCEPD pv tests #40885

copejon commented Feb 2, 2017

k8s-ci-robot commented Feb 2, 2017

k8s-reviewable commented Feb 2, 2017

copejon commented Feb 2, 2017

jeffvance Feb 2, 2017

copejon Feb 2, 2017

jeffvance Feb 2, 2017 •

edited

copejon Feb 2, 2017 •

edited

jeffvance Feb 3, 2017 •

edited

copejon Feb 13, 2017

madhusudancs commented Feb 2, 2017

rootfs Feb 2, 2017

rootfs Feb 2, 2017

jeffvance commented Feb 2, 2017

rootfs Feb 3, 2017

rootfs Feb 3, 2017

copejon Feb 3, 2017

rootfs commented Feb 3, 2017

copejon commented Feb 3, 2017

rootfs commented Feb 3, 2017

jeffvance Feb 21, 2017 •

edited

jeffvance commented Feb 21, 2017

copejon commented Feb 23, 2017

mikedanese commented Feb 23, 2017

copejon commented Feb 27, 2017

kerneltime commented Mar 2, 2017

jeffvance commented Mar 2, 2017

copejon commented Mar 13, 2017

jeffvance commented Mar 28, 2017

jeffvance commented Mar 28, 2017

k8s-github-robot commented Mar 28, 2017

k8s-github-robot commented Mar 28, 2017


		// Testing configurations of single a PV/PVC pair attached to a GCE PD
		var _ = framework.KubeDescribe("PersistentVolumes:GCEPD", func() {

Extract GCEPD pv tests #40885

Extract GCEPD pv tests #40885

Conversation

copejon commented Feb 2, 2017

k8s-ci-robot commented Feb 2, 2017

k8s-reviewable commented Feb 2, 2017

copejon commented Feb 2, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeffvance Feb 2, 2017 • edited

Choose a reason for hiding this comment

copejon Feb 2, 2017 • edited

Choose a reason for hiding this comment

jeffvance Feb 3, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

madhusudancs commented Feb 2, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeffvance commented Feb 2, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rootfs commented Feb 3, 2017

copejon commented Feb 3, 2017

rootfs commented Feb 3, 2017

jeffvance Feb 21, 2017 • edited

Choose a reason for hiding this comment

jeffvance commented Feb 21, 2017

copejon commented Feb 23, 2017

mikedanese commented Feb 23, 2017

copejon commented Feb 27, 2017

kerneltime commented Mar 2, 2017

jeffvance commented Mar 2, 2017

copejon commented Mar 13, 2017

jeffvance commented Mar 28, 2017

jeffvance commented Mar 28, 2017

k8s-github-robot commented Mar 28, 2017

k8s-github-robot commented Mar 28, 2017

jeffvance Feb 2, 2017 •

edited

copejon Feb 2, 2017 •

edited

jeffvance Feb 3, 2017 •

edited

jeffvance Feb 21, 2017 •

edited